Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Unset Content-Encoding header when uncompressed #1596

Merged
merged 2 commits into from
Aug 23, 2024
Merged

fix: Unset Content-Encoding header when uncompressed #1596

merged 2 commits into from
Aug 23, 2024

Conversation

mrueg
Copy link
Contributor

@mrueg mrueg commented Aug 23, 2024

This should fix the bug reported in #1595 as RFC2616 says not to set Content-Encoding header for responses without any encoding.

@ArthurSens @bwplotka

@mrueg mrueg force-pushed the fix-uncompressed-content-header branch from 2e4683c to 104aa34 Compare August 23, 2024 13:07
ArthurSens
ArthurSens approved these changes Aug 23, 2024
View reviewed changes
Copy link
Member

@ArthurSens ArthurSens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, I just tested locally and works as before :) Thanks for working on this

I wonder if we want to have a test that verifies that? Or too much?

bwplotka
bwplotka approved these changes Aug 23, 2024
View reviewed changes
Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I wished we have some safeguard for seeing content-encoding set AND accept-encoding causing us to do some different compression, but this is enough as a quick bug fix. Thanks a lot 💪🏽

@bwplotka bwplotka changed the base branch from main to release-1.20 August 23, 2024 15:37
@bwplotka
Copy link
Member

I changed the base to release branch as it has to be fixed there first, do you mind rebasing?

@bwplotka
Copy link
Member

We also need to add one commit for:
/VERSION set to 1.20.2
/CHANGELOG.md section for new release and current bugfix

But I can help do that

@mrueg mrueg force-pushed the fix-uncompressed-content-header branch from 104aa34 to 187acd4 Compare August 23, 2024 16:55
@ArthurSens ArthurSens merged commit 67121dc into prometheus:release-1.20 Aug 23, 2024
8 checks passed
@ArthurSens ArthurSens mentioned this pull request Aug 23, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ArthurSens ArthurSens ArthurSens approved these changes

@bwplotka bwplotka bwplotka approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mrueg @bwplotka @ArthurSens